Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Get metadata from scicat using token #353

Merged
merged 6 commits into from
Mar 17, 2024
Merged

Get metadata from scicat using token #353

merged 6 commits into from
Mar 17, 2024

Conversation

kutnyakhov
Copy link
Collaborator

Due to update of SciCat to a new backend at the beginning of the year, not only headers were changed but also URL was corrected (one "/" in front was removed).
Thus, it was possible to get only metadata ingested into SciCat this year using last PR #347 .
This PR now checking if "requests.get(new URL)" returns empty object and then uses instead "requests.get(old URL)".

@kutnyakhov kutnyakhov requested a review from zain-sohail March 5, 2024 13:23
@zain-sohail zain-sohail force-pushed the get-metadata-from-scicat-using-token branch from 12fde6b to 6a535f0 Compare March 8, 2024 06:54
zain-sohail
zain-sohail approved these changes Mar 17, 2024
View reviewed changes
Copy link
Member

@zain-sohail zain-sohail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested that data is being loaded so I will merge it now.

@zain-sohail zain-sohail merged commit e1e5ca1 into main Mar 17, 2024
6 checks passed
@zain-sohail zain-sohail deleted the get-metadata-from-scicat-using-token branch March 17, 2024 11:23
@rettigl
Copy link
Member

rettigl commented Mar 17, 2024

I have tested that data is being loaded so I will merge it now.

Did you check that all example code work without modification? This pr introduces a new config key without changing any config file, nor any appropriate error handling as it appears to me.

@zain-sohail
Copy link
Member

I have tested that data is being loaded so I will merge it now.

Did you check that all example code work without modification? This pr introduces a new config key without changing any config file, nor any appropriate error handling as it appears to me.

The key is only necessary if user gives the keyword argument collect_metadata to true.
Even if it is true, and no relevant config key appears, the code still runs and a warning is sent to user.
Eitherways this pr just introduces the token based system, because the username and password is depreciated at scicat. So even for old beam times, only this version can work. Hence the urgency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zain-sohail zain-sohail zain-sohail approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kutnyakhov @rettigl @zain-sohail